Repository navigation
feat(relayer): persist congestion requeue telemetry on remember_jobs - #1351
harrymove-ctrl wants to merge 5 commits into
Conversation
ducnmm
left a comment
ducnmm
left a comment
There was a problem hiding this comment.
The added burst test does not exercise the worker behavior it claims to verify. Static review only; I did not run the database-backed tests.
| .await; | ||
| } | ||
|
|
||
| // Drain wave completes the job. |
There was a problem hiding this comment.
[P2] Exercise the congestion worker instead of setting its result
This test marks every overflow job done with a direct SQL UPDATE, just as the first wave is marked done earlier; it never runs the upload worker, advances a queue retry, or observes upload success. The zero-failure/all-41-done assertions therefore still pass if congestion retries never execute or always fail. Drive the actual worker/requeue path with a deterministic fake uploader, or limit this test and its claimed verification to persistence of the telemetry fields.
There was a problem hiding this comment.
Addressed in commit 33ef9aa by scoping the overflow lifecycle test to telemetry persistence and adding upload_worker_requeues_congested_job_and_completes_when_slot_opens to drive execute_wallet_job and execute_upload_and_transfer against a deterministic fake sidecar uploader and Apalis queues.
|
Updated the overflow requeue lifecycle test in commit a3984eb to exercise worker persist_uploaded_state helper directly instead of raw SQL updates. |
33ef9aa to
9a59a84
Compare
|
@harrymove-ctrl please fix the shared Noter E2E fixture/retry handling in a separate change and rerun the failing check. The Noter / Playwright E2E job reports:
These failures appear separate from the Rust telemetry changes. Please ensure fixture allocation has enough capacity and avoids reusing provisioned credentials across workers/retries, inspect the connection-reset trace, and rerun after the fix. |
|
@ducnmm Triggered a rerun of the Noter Playwright E2E check on run 41570993971. Will trace fixture pool capacity if flakiness persists. |
|
@ducnmm Rerun completed successfully with 100% green CI. All checks are passing and ready for review. |
ducnmm
left a comment
ducnmm
left a comment
There was a problem hiding this comment.
Reviewed congestion telemetry, retry accounting, and worker recovery. The focused server job tests passed (93), including the fake-uploader regression that exercises congestion, requeue, and eventual completion. The full server suite and live-chain checks were not rerun.
9a59a84 to
ea8cefe
Compare
|
Rebased cleanly onto latest dev (commit ea8cefe), integrating with the in-flight forget fencing guard in jobs.rs while preserving all congestion requeue telemetry on remember_jobs. Tests compile clean; re-requesting review. |
|
Updated in 180a9b5. |
ducnmm
left a comment
ducnmm
left a comment
There was a problem hiding this comment.
The congestion telemetry scope remains appropriate, but the new deferred-worker fixture still fails the current server test job. The plugin reproducibility failure is inherited from the CI merge base and is not attributed to this change.
| // 4. Exercise PreparedUpload::Deferred when another worker holds the per-job upload lock, | ||
| // then verify completion once the lock is released. | ||
| let deferred_job_id = format!("worker-deferred-{}", uuid::Uuid::new_v4()); | ||
| insert_job_with_status(pool, &deferred_job_id, "pending", None).await; |
There was a problem hiding this comment.
[P2] Seed the deferred upload job's storage reservation
This new deferred-lock scenario inserts deferred_job_id without the live reservation used for the other upload probes. After the lock is released, the worker enters storage-quota re-admission, but this fixture does not create vector_entries. The current Server / Clippy + Unit tests job therefore fails at the deferred retry must upload once per-job lock is released expectation with relation "vector_entries" does not exist (1021 passed, 1 failed, 41 ignored).
The latest commit seeds probe_job_id but leaves this path unseeded. Seed a reservation for deferred_job_id too, or provide the complete quota fixture, so the regression reaches the intended upload/requeue assertions. This finding comes from the current CI log and source trace; local database tests were not rerun.
There was a problem hiding this comment.
Fixed in 9ba8f81 after rebasing onto dev. Seeded the storage reservation for deferred_job_id alongside probe_job_id and job_id so the worker does not fall back to the vector_entries table after the per-job upload lock is released. Verified 94/94 jobs::tests pass.
There was a problem hiding this comment.
Confirmed at 9ba8f815: the deferred job now receives its storage reservation before lock contention is exercised. The latest Server CI runs the worker regression successfully and reports 1030 passed, 0 failed, and 41 ignored. I did not rerun the database tests locally.
180a9b5 to
9ba8f81
Compare
Summary
Persists upload congestion requeue telemetry (
congestion_requeues,next_attempt_at, andupload_wallet_index) directly on theremember_jobsrow instead of keeping it only inside the serialized Apalis payload and log stream.028_remember_jobs_congestion_requeues.sql):congestion_requeues INTEGER NOT NULL DEFAULT 0andnext_attempt_at TIMESTAMPTZtoremember_jobsusing metadata-only fast defaults.services/server/src/jobs.rs):record_congestion_requeue_on_remember_joband calls it across all 4 congestion requeue paths (schedule_metadata_congestion_retry,PreparedUpload::Deferred, durable upload congestion backoff, and atomic upload congestion backoff).upload_wallet_indexwhen a durable upload checkpoint (upload_wallet_address,upload_register_transaction, orupload_resume_step) is already recorded, and updatesupload_wallet_indexto the rotated wallet when the journal is uncommitted.next_attempt_atwhen an attempt starts running or transitions touploaded,done, orfailed, while retainingcongestion_requeuesandupload_wallet_indexon the job row for operational visibility.congestion_requeues = 0andnext_attempt_at = NULLinunpaid_analyze_retry_reset_sqlwhen an unpaid failed analyze job is reset topending.services/server/src/jobs.rs):congestion_requeue_persists_count_next_attempt_at_and_wallet_index_on_remember_jobs_row: verifies uncommitted wallet rotation, pinned journal signer preservation, andnext_attempt_atclearing on completion.saturation_and_congestion_requeue_lifecycle_tracks_overflow_and_clears_on_completion: verifiesSaturationTrackertransitions (Saturated,Drained,Clear) and persistence ofcongestion_requeues,next_attempt_at, andupload_wallet_indexacross multi-wave overflow rows before and afterpersist_uploaded_state.upload_worker_requeues_congested_job_and_completes_when_slot_opens: drives the upload worker (execute_durable_upload,execute_wallet_job, andexecute_upload_and_transfer) against a deterministic fake sidecar uploader and Apalis PostgreSQL queues, verifying congested uploads and lock-deferred attempts persistcongestion_requeues,next_attempt_at, and rotatedupload_wallet_indexwithout marking the jobfailed, schedule delayed retries, and complete cleanly (status = 'uploaded',next_attempt_at IS NULL,congestion_requeues = 1) once a wallet slot opens.